components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/util/FrameworkUtils.java (1)

1466-1471: ⚡ Quick win

Log tenant-scoped session cache misses.

This new overload is the tenant-aware session lookup path; when the explicit tenant scope misses, the cross-org SSO flow silently falls back to re-auth. Add a guarded DEBUG log that avoids session keys and user data. As per coding guidelines, significant auth/session operations should include safe, non-repetitive logs.

Suggested logging addition

             SessionContextCacheEntry cacheEntry = sessionContextCache.getSessionContextCacheEntry(cacheKey,
                     tenantDomain);
 
             if (cacheEntry != null) {
                 sessionContext = cacheEntry.getContext();
                 boolean isSessionExpired = sessionContextCache.isSessionExpired(cacheKey, cacheEntry);
                 if (isSessionExpired) {
                     triggerSessionExpireEvent(request, context, sessionContext);
                     if (log.isDebugEnabled()) {
                         log.debug("A SESSION_EXPIRE event was fired for the expired session found corresponding " +
                                 "to the key: " + cacheKey.getContextId());
                     }
                     return null;
                 }
+            } else if (log.isDebugEnabled()) {
+                log.debug("No session context cache entry found for tenant domain: " + tenantDomain);
             }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/util/FrameworkUtils.java`
around lines 1466 - 1471, Add a guarded DEBUG log to handle tenant-scoped
session cache misses in the SessionContextCache lookup. After the call to
sessionContextCache.getSessionContextCacheEntry(cacheKey, tenantDomain), check
if the returned cacheEntry is null. When it is null, log a DEBUG-level message
(guarded with a check to see if DEBUG logging is enabled) indicating a session
cache miss has occurred. The log message should be descriptive about the cache
miss without including sensitive data like session keys or user identifiers,
following the pattern of safe non-repetitive logging for significant
authentication and session operations.

Source: Coding guidelines

components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/handler/request/impl/DefaultRequestCoordinator.java (1)

1642-1644: ⚡ Quick win

Log when org discovery input disables session reuse.

This is a significant auth/SSO decision point, but it returns silently. Add a short DEBUG log before returning so discovery-driven skips are diagnosable without logging request values. As per coding guidelines, “Place logs around significant decision points/functional branches in the auth/SSO/session-merging flows.”

🪵 Proposed log

         if (hasOrganizationDiscoveryParameters(request)) {
+            if (log.isDebugEnabled()) {
+                log.debug("Organization discovery parameters found. Skipping previous organization session reuse.");
+            }
             return Optional.empty();
         }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/handler/request/impl/DefaultRequestCoordinator.java`
around lines 1642 - 1644, In the DefaultRequestCoordinator class, the
conditional block checking hasOrganizationDiscoveryParameters(request) silently
returns Optional.empty() without logging. Add a DEBUG level log statement before
the return to document that organization discovery parameters are preventing
session reuse. The log should provide context about this significant auth/SSO
decision point without including request values, making the flow diagnosable
during troubleshooting.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/util/FrameworkUtils.java (1)

1342-1349: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add a guarded DEBUG log for cache-only session writes.

This new method performs a key session-cache write path, but there’s no local trace point for tenant/org routing during troubleshooting.

♻️ Suggested update

 public static void addSessionContextToCacheWithoutPersisting(String key, SessionContext sessionContext,
                                                              String tenantDomain, String loginTenantDomain,
                                                              String orgId) {
 
     SessionContextCacheKey cacheKey = new SessionContextCacheKey(key);
     SessionContextCacheEntry cacheEntry = buildSessionContextCacheEntry(key, sessionContext, tenantDomain, orgId);
+    if (log.isDebugEnabled()) {
+        log.debug("Caching session context without persistence. key: " + key + ", loginTenantDomain: " +
+                loginTenantDomain + ", orgId: " + orgId);
+    }
     SessionContextCache.getInstance().addToCacheWithoutPersisting(cacheKey, cacheEntry, loginTenantDomain);
 }

As per coding guidelines, “flag functions that perform significant operations but lack log statements” and “add logs around major method executions.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/util/FrameworkUtils.java`
around lines 1342 - 1349, The method addSessionContextToCacheWithoutPersisting
performs a significant session-cache write operation but lacks any logging for
troubleshooting purposes. Add a guarded DEBUG log statement at the beginning of
this method that logs relevant tenant and organization routing information. The
log should include the key, tenantDomain, loginTenantDomain, and orgId
parameters to provide visibility into cache-only session writes during
debugging. Use a guard condition to check if DEBUG logging is enabled before
logging to avoid performance overhead.

Source: Coding guidelines

components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/handler/request/impl/DefaultAuthenticationRequestHandler.java (1)

890-893: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Use WARN instead of ERROR for recoverable tenant domain resolution failures.

The system continues processing other organizations when this exception occurs, making this a recoverable deviation rather than a feature failure. Per logging guidelines, WARN is appropriate for "deviations/expected-but-problematic cases where the system continues."

♻️ Suggested fix

             } catch (OrganizationManagementException e) {
-                log.error("Error while resolving the tenant domain of the organization with id: " +
+                log.warn("Error while resolving the tenant domain of the organization with id: " +
                         authenticatedOrgId, e);
             }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/handler/request/impl/DefaultAuthenticationRequestHandler.java`
around lines 890 - 893, The OrganizationManagementException catch block in
DefaultAuthenticationRequestHandler.java is using log.error() for a recoverable
failure case where the system continues processing other organizations. Change
the log.error() call to log.warn() since WARN level is appropriate for
deviations where the system continues operating, and ERROR should be reserved
for actual feature failures.

Source: Coding guidelines

components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/test/java/org/wso2/carbon/identity/application/authentication/framework/handler/request/impl/DefaultRequestCoordinatorTest.java (1)

1115-1131: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add explicit coverage for the UserIdNotFoundException branch.

Line 1115 currently validates only the OrganizationManagementException path, but isAuthenticatedUserSharedToAccessingOrg(...) also handles UserIdNotFoundException by returning false. Add a sibling test to pin that behavior and prevent regressions.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/test/java/org/wso2/carbon/identity/application/authentication/framework/handler/request/impl/DefaultRequestCoordinatorTest.java`
around lines 1115 - 1131, The test
`testIsAuthenticatedUserSharedToAccessingOrgOnException` only covers the
OrganizationManagementException path, but the
`isAuthenticatedUserSharedToAccessingOrg` method also handles
UserIdNotFoundException by returning false. Add a new sibling test method
following the same pattern as
`testIsAuthenticatedUserSharedToAccessingOrgOnException` that instead mocks the
`OrganizationUserSharingService` to throw a `UserIdNotFoundException` when
`getUserAssociationOfAssociatedUserByOrgId` is called, verifying that the method
still returns false and the exception is properly caught and handled.

components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/handler/request/impl/DefaultRequestCoordinator.java (1)

1640-1643: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider adding a DEBUG log when skipping session reuse due to org discovery parameters.

This decision point affects SSO behavior and a DEBUG log would aid troubleshooting when session reuse is unexpectedly skipped.

📝 Suggested log addition

         if (hasOrganizationDiscoveryParameters(request)) {
+            if (log.isDebugEnabled()) {
+                log.debug("Skipping organization session reuse due to presence of organization discovery parameters.");
+            }
             return Optional.empty();
         }

Based on learnings: Per LOG_ENHANCEMENT_GUIDELINES.md, DEBUG logs should be added around meaningful business logic decisions and guarded with isDebugEnabled().

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/handler/request/impl/DefaultRequestCoordinator.java`
around lines 1640 - 1643, Add a DEBUG log statement before the return
Optional.empty() call in the hasOrganizationDiscoveryParameters check block.
Guard the log with isDebugEnabled() to avoid unnecessary string construction,
and include a meaningful message that explains the session reuse is being
skipped due to organization discovery parameters. This will help with
troubleshooting when SSO behavior is unexpectedly affected by this decision
point.

Source: Coding guidelines